Add local debug options as a dependency, but only if local_debug_options_enabled #98

amberdixon · 2020-08-04T22:24:01Z

Only do this if local_debug_options_enabled is specified at command line. Make xcode specify this config.

The changes in this PR are taken from the suggested changes in https://github.com/ios-bazel-users/ios-bazel-users/blob/master/DebuggableRemoteSwift.md to get debugging working.

Note that in order for the example applications and tests in the rules_ios repo to be debuggable from Xcode, you must:

Disable sandboxing in .bazelrc.
Update the flag in the tools/xcodeproj_shims/build-wrapper.sh to refer to --//rules:local_debug_options_enabled rather than --@build_bazel_rules_ios//rules:local_debug_options_enabled

There are still known issues with evaluating variables from the debugger for swift tests.

rules/library.bzl

segiddins · 2020-08-04T22:28:25Z

tests/macos/xcodeproj/build.sh

+xcodebuild -project Test-Target-With-Test-Host-Project.xcodeproj -quiet
+# TODO: use strings to test that absolute paths and debug-prefix-map are present in the LocalDebug.swiftmodule. Not sure if this is the right palce to do it. We could also grep out the build event text file maybe.


trailing newline

Nit: spelling of place

segiddins · 2020-08-04T22:42:39Z

rules/app.bzl

@@ -56,7 +56,7 @@ def ios_application(name, apple_library = apple_library, **kwargs):
    """
    infoplists = write_info_plists_if_needed(name = name, plists = kwargs.pop("infoplists", []))
    application_kwargs = {arg: kwargs.pop(arg) for arg in _IOS_APPLICATION_KWARGS if arg in kwargs}
-    library = apple_library(name = name, namespace_is_module_name = False, platforms = {"ios": application_kwargs.get("minimum_os_version")}, **kwargs)
+    library = apple_library(name = name, namespace_is_module_name = False, platforms = {"ios": application_kwargs.get("minimum_os_version")}, add_swift_local_debug_options = True, **kwargs)


should also be passed for tests as well?

Alternatively, we could only add in the select to the deps passed to rules_apple_ios_application , and make it conditional on whether there were actually any swift files? That way, the swiftmodule that represents the app needn't be invalidated by switching the flag either

as of Xcode 11.5, we don't need/use this workaround for apps, only tests.

…options_enabled is specified at command line. Make xcode specify this config

segiddins

Looks good! Does of of the test projects in this repo demonstrate the fix?

segiddins · 2020-08-10T16:07:24Z

rules/BUILD.bazel

+            "-serialize-debugging-options",
+    ],
+    module_name = "_LocalDebugOptions",
+    tags = ["no-remote"],


Nit: can we also tag as manual?

segiddins · 2020-08-10T16:08:33Z

tests/macos/xcodeproj/build.sh

+xcodebuild -project Test-Target-With-Test-Host-Project.xcodeproj -quiet
+# TODO: use strings to test that absolute paths and debug-prefix-map are present in the LocalDebug.swiftmodule. Not sure if this is the right palce to do it. We could also grep out the build event text file maybe.


Nit: spelling of place

amberdixon · 2020-08-10T16:31:05Z

I am still seeing issues with test bundles locally. I get errors like this. This happens for tests written in swift both with and without an app host:

AppRoot-Unit-Tests: error: cannot load underlying module for 'XCTest'

Debug info from this module will be unavailable in the debugger.

warning: Swift error in scratch context: error: missing required modules:

That said, I think the changes in this PR are still correct, but perhaps we need to do a few more things in terms of setting additional lldb settings. Going to open this up for review and then continue working on these issues I'm seeing with tests locally.

segiddins · 2020-08-10T20:24:06Z

AppRoot-Unit-Tests: error: cannot load underlying module for 'XCTest'

We might need to add in $(PLATFORM_DIR)/Developer/Library/Frameworks to FRAMEWORK_SEARCH_PATHS for test target types?

gyfelton

LGTM 👍

gyfelton · 2020-08-11T18:47:54Z

tools/xcodeproj_shims/build-wrapper.sh

@@ -1,4 +1,4 @@
 #!/bin/bash
 set -euxo pipefail

-$BAZEL_PATH build --experimental_execution_log_file=$BAZEL_BUILD_EXECUTION_LOG_FILENAME --build_event_text_file=$BAZEL_BUILD_EVENT_TEXT_FILENAME --build_event_publish_all_actions $1 2>&1 | $BAZEL_OUTPUT_PROCESSOR
+$BAZEL_PATH build --experimental_execution_log_file=$BAZEL_BUILD_EXECUTION_LOG_FILENAME --build_event_text_file=$BAZEL_BUILD_EVENT_TEXT_FILENAME --build_event_publish_all_actions $1 --@build_bazel_rules_ios//rules:local_debug_options_enabled 2>&1 | $BAZEL_OUTPUT_PROCESSOR


run the following to get test fixtures updated:

bazelisk query 'kind(xcodeproj, tests/ios/xcodeproj/...)' | xargs -n 1 bazelisk run bazelisk query 'kind(xcodeproj, tests/macos/xcodeproj/...)' | xargs -n 1 bazelisk run bazelisk query 'attr(executable, 1, kind(genrule, tests/macos/xcodeproj/...))' | xargs -n 1 bazelisk run bazelisk query 'attr(executable, 1, kind(genrule, tests/ios/xcodeproj/...))' | xargs -n 1 bazelisk run find . -type f $ -name 'WORKSPACE' -o -name '*.bzl' -o -name '*.bazel' $ | xargs buildifier -lint=fix

gyfelton · 2020-08-11T18:48:41Z

rules/app.bzl

@@ -61,9 +61,16 @@ def ios_application(name, apple_library = apple_library, **kwargs):
    application_kwargs["launch_storyboard"] = application_kwargs.pop("launch_storyboard", library.launch_screen_storyboard_name)
    application_kwargs["families"] = application_kwargs.pop("families", ["iphone", "ipad"])

+    local_debug_options_for_swift = []
+    if library.has_swift_sources:
+      local_debug_options_for_swift += ["@build_bazel_rules_ios//rules:_LocalDebugOptions"]


should we update some documentation / readme to reflect this important flag?

I added comments in the BUILD.bazel where the _LocalDebugOptions target is defined. I will also reference this README: https://github.com/ios-bazel-users/ios-bazel-users/blob/master/DebuggableRemoteSwift.md

Adding separate docs shouldn't be necessary, since the xcodeproj rule automatically enables the flag when necessary

segiddins · 2020-08-11T23:18:38Z

.bazelrc

+# Enable these features to test local and CI builds
+# when swiftmodule caching is enabled.
+build --features=swift.cacheable_swiftmodules
+build --features=swift.use_global_module_cache


nit: trailing newline

segiddins · 2020-08-11T23:23:09Z

tests/macos/xcodeproj/build.sh

nit: trailing newline

amberdixon requested review from segiddins and gyfelton August 4, 2020 22:24

segiddins reviewed Aug 4, 2020

View reviewed changes

Add local debug options as a dependency, but only if the local_debug_…

e230690

…options_enabled is specified at command line. Make xcode specify this config

amberdixon force-pushed the amber/local-debug-options branch from bcca942 to e230690 Compare August 10, 2020 15:47

segiddins reviewed Aug 10, 2020

View reviewed changes

gyfelton approved these changes Aug 11, 2020

View reviewed changes

amberdixon added 2 commits August 11, 2020 16:05

Improvements to debugging, add comments and PR feedback

2556a04

rerun tests, add some more comments

3e82f9b

amberdixon marked this pull request as ready for review August 11, 2020 23:18

segiddins approved these changes Aug 11, 2020

View reviewed changes

add newline. no reason to keep caching enabled for this repo

2b769bd

amberdixon merged commit 359607a into master Aug 12, 2020

segiddins deleted the amber/local-debug-options branch August 12, 2020 00:39

gyfelton mentioned this pull request Aug 14, 2020

Force swift local debug option to be turned on so that framework search path has correct payload after project is built #101

Merged

amberdixon mentioned this pull request Aug 26, 2020

[xcodeproj] Local breakpoints do not work when sandbox is enabled #108

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add local debug options as a dependency, but only if local_debug_options_enabled #98

Add local debug options as a dependency, but only if local_debug_options_enabled #98

amberdixon commented Aug 4, 2020 •

edited

Loading

segiddins Aug 4, 2020

segiddins Aug 10, 2020

segiddins Aug 4, 2020

segiddins Aug 4, 2020

kastiglione Aug 4, 2020 •

edited

Loading

segiddins left a comment

segiddins Aug 10, 2020

segiddins Aug 10, 2020

amberdixon commented Aug 10, 2020

segiddins commented Aug 10, 2020

gyfelton left a comment

gyfelton Aug 11, 2020

gyfelton Aug 11, 2020

amberdixon Aug 11, 2020

segiddins Aug 11, 2020

segiddins Aug 11, 2020

segiddins Aug 11, 2020

		xcodebuild -project Test-Target-With-Test-Host-Project.xcodeproj -quiet
		# TODO: use strings to test that absolute paths and debug-prefix-map are present in the LocalDebug.swiftmodule. Not sure if this is the right palce to do it. We could also grep out the build event text file maybe.

Add local debug options as a dependency, but only if local_debug_options_enabled #98

Add local debug options as a dependency, but only if local_debug_options_enabled #98

Conversation

amberdixon commented Aug 4, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kastiglione Aug 4, 2020 • edited Loading

Choose a reason for hiding this comment

segiddins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amberdixon commented Aug 10, 2020

segiddins commented Aug 10, 2020

gyfelton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amberdixon commented Aug 4, 2020 •

edited

Loading

kastiglione Aug 4, 2020 •

edited

Loading